fix: prevent crashes when parsing Rust/Swift files (#6532) #6533
Conversation
…rror handling - Add timeout protection for WASM loading to prevent hanging - Return null instead of throwing when WASM fails to load - Add try-catch blocks around parser operations - Implement memory management to clean up old parsers - Add timeout for parsing operations to handle complex files This fixes issue #6532 where Roo Code crashes repeatedly, especially with Rust and Swift code on macOS.
dosubot
bot
added
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| function startCleanupInterval() { | ||
| if (!cleanupInterval) { | ||
| cleanupInterval = setInterval(cleanupOldParsers, PARSER_CLEANUP_INTERVAL) | ||
| } |
There was a problem hiding this comment.
Critical issue: The cleanup interval is never stopped when the extension deactivates. This could cause memory leaks as the interval continues running even after the extension is disabled.
Consider adding a mechanism to stop the interval on extension deactivation, perhaps by exposing a shutdown function that calls stopCleanupInterval().
| } | ||
| }) | ||
|
|
||
| const tree = await Promise.race([parsePromise, parseTimeout]) |
There was a problem hiding this comment.
Is this approach intentional? Using Promise.race here means the parser might continue running in the background even after the timeout resolves. This could lead to resource consumption.
Consider using AbortController or a similar mechanism to actually cancel the parsing operation when the timeout occurs.
| console.error(`Error loading language: ${wasmPath}: ${error instanceof Error ? error.message : error}`) | ||
| throw error | ||
| // Return null instead of throwing to prevent crashes | ||
| return null |
There was a problem hiding this comment.
The error handling approach differs between loadLanguage() (returns null) and loadRequiredLanguageParsers() (continues execution). Could we standardize this for consistency?
Also, returning null here might mask important errors. Consider differentiating between expected failures (unsupported language) and unexpected errors.
|
|
||
| // Add timeout for WASM loading to prevent hanging | ||
| const loadTimeout = new Promise((_, reject) => { | ||
| setTimeout(() => reject(new Error(`Timeout loading WASM file: ${wasmPath}`)), 30000) |
There was a problem hiding this comment.
These timeout values (30s, 10s, 60s) are hardcoded throughout. Would it make sense to extract them as configurable constants at the module level? This would make it easier to tune performance based on different environments.
| } | ||
|
|
||
| // Export cleanup function for testing and manual cleanup | ||
| export function cleanupAllParsers() { |
There was a problem hiding this comment.
The new timeout functionality, memory management, and error handling paths don't appear to have test coverage. Given that this is critical functionality for preventing crashes, could we add tests to ensure these mechanisms work as expected?
| // Parse the file content into an Abstract Syntax Tree (AST) | ||
| const tree = parser.parse(fileContent) | ||
| // Add timeout to prevent hanging on large or complex files | ||
| const parseTimeout = new Promise<any>((resolve) => { |
There was a problem hiding this comment.
Using any type here reduces type safety. Could we define proper types for the tree structure to maintain type safety throughout the Promise chain?
| } | ||
|
|
||
| // Export cleanup function for testing and manual cleanup | ||
| export function cleanupAllParsers() { |
There was a problem hiding this comment.
This cleanup function is exported but doesn't seem to be integrated into the extension's deactivation lifecycle. Should this be called when the extension deactivates to ensure proper cleanup?
hannesrudolph
added
the
Issue/PR - Triage
|
Closing for now, the issue is not properly scoped yet |
Summary
This PR fixes issue #6532 where Roo Code crashes repeatedly, especially when working with Rust and Swift files on macOS.
Problem
The tree-sitter WASM parser was causing VSCode to crash when:
Solution
1. Robust WASM Loading
2. Safe Parsing Operations
3. Memory Management
Testing
Related Issues
Fixes #6532
Checklist
Important
Fixes crashes by adding timeouts and error handling for WASM parser loading and parsing operations, and implements memory management for parsers.
loadLanguage()inlanguageParser.tsto prevent hanging.parseFile()inindex.tswith a 10s timeout and try-catch to handle errors gracefully.languageParser.tswith automatic cleanup of unused parsers after 1 minute.loadRequiredLanguageParsers()inlanguageParser.tsto handle unsupported languages gracefully.This description was created by
for fdc3e6a. You can customize this summary. It will automatically update as commits are pushed.